Skip to content

add a test case for issue #32031 #34241

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 12, 2016
Merged

Conversation

dsprenkels
Copy link
Contributor

I propose a test case to finish the fix for issue #32031. Please review this commit thoroughly, as I have never written a codegen test before.

r? @eddyb

@eddyb
Copy link
Member

eddyb commented Jun 12, 2016

@dsprenkels Can you add the email address you used in the commit to your github account? Otherwise the commits are not linked to your account. This is of course not necessary, let me know if it's intentional.

#[no_mangle]
pub struct F32(f32);

// CHECK: @add_newtype_f32(float, float)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any chance this could also check the return type?

Copy link
Member

@eddyb eddyb Jun 12, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I meant, btw, is that the full signature is define float @add_newtype_f32(float, float) - and if you include all of it, you also ensure the return type is correct.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, check. I'll fix this accordingly.

@dsprenkels
Copy link
Contributor Author

dsprenkels commented Jun 12, 2016

Yup, this was indeed not intentional. Thanks for the heads-up!
(I did this commit on my server, where compilation is a lot faster.)

I also added a check for the return type.

pub struct F64(f64);

// CHECK: @add_newtype_f64(double, double)
// CHECK: ret float
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this not double?

@eddyb
Copy link
Member

eddyb commented Jun 12, 2016

@bors r+ Thanks!

@bors
Copy link
Collaborator

bors commented Jun 12, 2016

📌 Commit 688840f has been approved by eddyb

@bors
Copy link
Collaborator

bors commented Jun 12, 2016

⌛ Testing commit 688840f with merge abc57ab...

bors added a commit that referenced this pull request Jun 12, 2016
add a test case for issue #32031

I propose a test case to finish the fix for issue #32031. Please review this commit thoroughly, as I have never written a codegen test before.

r? @eddyb
@bors bors merged commit 688840f into rust-lang:master Jun 12, 2016
@dsprenkels dsprenkels deleted the issue-32031-test branch June 12, 2016 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants